Improve performance of basic incidence matrix computation #946
Conversation
| push!(I, i); push!(J, branch["f_bus"]); push!(V, 1) | ||
| push!(I, i); push!(J, branch["t_bus"]); push!(V, -1) | ||
| for e in 1:E | ||
| branch = data["branch"]["$e"] |
There was a problem hiding this comment.
type annotation here didn't seem to yield any improvement, so I didn't include it to keep the code simple.
|
What about E, N = length(data["branch"])::Int, length(data["bus"])::Int
# I = [..., e, e, ...]
I = repeat(1:E; inner = 2)
J = zeros(Int, 2 * E)
for e in 1:E
branch = data["branch"]["$e"]
J[2e-1] = branch["f_bus"]::Int
J[2e] = branch["t_bus"]::Int
end
# V = [..., 1, -1, ...]
V = repeat([1, -1]; inner = 2) |
|
Thanks! This gives a nice extra 10% performance boost :) BenchmarkTools.Trial: 517 samples with 1 evaluation per sample.
Range (min … max): 8.979 ms … 20.571 ms ┊ GC (min … max): 0.00% … 48.89%
Time (median): 9.165 ms ┊ GC (median): 0.00%
Time (mean ± σ): 9.671 ms ± 1.367 ms ┊ GC (mean ± σ): 3.19% ± 8.24%
██▅ ▃▂▁ ▄▃
███████▅▆▆██▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▄██▅▄▅▁▅▆▁▁▄▁▄▁▁▁▁▁▁▁▄▁▁▄▄ ▇
8.98 ms Histogram: log(frequency) by time 15.7 ms <
Memory estimate: 2.84 MiB, allocs estimate: 32135. |
|
Updates look good to me. Just need to add a note to the change log and merge. Open to ideas for adding a test, but also know it can be very hard to accurately capture runtime performance in CI. |
|
Change log updated --> this should go into the "staged" section, correct? |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #946 +/- ##
=======================================
Coverage 93.71% 93.71%
=======================================
Files 43 43
Lines 9636 9636
=======================================
Hits 9030 9030
Misses 606 606
Continue to review full report in Codecov by Sentry.
|
A substantial amount of time is spent in this line
PowerModels.jl/src/core/data_basic.jl
Line 250 in 8824212
mainly because the
sortdoes two dictionary lookups every time is needs to compare two branches.This is un-necessary for two reasons:
1toE(number of branches) and no disabled branchessparsewill re-sort any non-ordered indices if neededRemoving this step, plus some type annotations to help the compiler, result in a substantial performance improvement.
note that relative gains get better as system size increases.
Performance benchmark on the
9241_pegasecase from PGLibBefore
After